GH-39784: [C++][Gandiva] Fix decimal in_expr crash with cached object code#49951
Open
Reranko05 wants to merge 1 commit intoapache:mainfrom
Open
GH-39784: [C++][Gandiva] Fix decimal in_expr crash with cached object code#49951Reranko05 wants to merge 1 commit intoapache:mainfrom
Reranko05 wants to merge 1 commit intoapache:mainfrom
Conversation
|
|
ed00934 to
f6ce3bf
Compare
f6ce3bf to
5805fc6
Compare
Contributor
Author
|
The CI failed job looks unrelated to the Gandiva changes in this PR. The failure is in a PyArrow S3 filesystem test on the macOS Intel Python job, while the Gandiva tests passed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale for this change
The decimal-specific
VisitInExpressionLLVM generation path embedded a raw holder pointer directly into generated object code. When Gandiva reused cached object code, the embedded pointer could become stale, leading to crashes during cached decimalin_exprevaluation.What changes are included in this PR?
in_exprLLVM generation to load holder pointers dynamically at runtime usingarg_holder_ptrs_, matching the existing genericInExpressionimplementation.in_exprexecution.Are these changes tested?
Yes.
TestIn.TestInDecimalCachedTestIn.*--gtest_repeat=100Are there any user-facing changes?
No.
This PR contains a "Critical Fix".
This change fixes a cache-path crash in Gandiva when evaluating decimal
in_exprexpressions using cached object code.GitHub Issue: #39784